Conversation
675809d to
428d0c3
Compare
|
Subject: Context for the regex adjustment Following up on this one: this patch allows for casing variations (e.g., .AppImage vs .appimage) and version-string formatting that were causing an install failure on my Void Linux setup. This works alongside the fix you just merged in #1455 to get the installer running here. Thanks for merging that one! Happy to refine the pattern if you have a preferred regex style. |
| do_install_appimage() { | ||
| log_info "Attempting AppImage install..." | ||
| if ! check_cmd curl; then log_error "curl is required."; fi | ||
| if ! check_cmd curl; then log_error "curl is required."; return 1; fi |
There was a problem hiding this comment.
log_error already exits, no need for return
| fi | ||
|
|
||
| log_info "Finalizing installation..." | ||
| mkdir -p "$INSTALL_DIR" "$BIN_DIR" |
There was a problem hiding this comment.
INSTALL_DIR is immediately removed after being created here, can skip creation
| rm -rf "$TEMP_EXTRACT" | ||
|
|
||
| # Atomic update of the installation directory | ||
| rm -rf "$INSTALL_DIR" |
There was a problem hiding this comment.
potentially destroys user data? can we move to tmp dir instead?
sinelaw
left a comment
There was a problem hiding this comment.
A few comments, please take a look
Problem
The AppImage installer currently fails silently if
mktemporcurlfails, reporting a "Success" message even if no files were moved. Additionally, it fails on systems with restricted/tmppermissions (such as Void Linux or certain containers).Solution
This PR introduces several robustness improvements to ensure a clean and honest installation process:
trapto ensure the temporary workspace is deleted on exit or interruption.$HOME/.cacheif the system/tmpis not writable.curl,mktemp, andmvto prevent "fake success" messages.Testing
Verified on Void Linux x86_64. The script now correctly falls back to local storage when
/tmpis restricted and successfully completes the installation.